Could you explain why you chose for 300px? Is the behavior the same as YT? If not why is this better?

Could you explain why you chose for 300px? Is the behavior the same as YT? If not why is this better?

I chose 300px because that's the height/max-block-size the description was already set to (look at .videoDescription in WatchVideoDescription.css).

My (purely visual, untested) feedback: I like YT's UI for this better, particularly that the click area is the full description box so it's easier to click, the "more" button is casual, placed inside the flow of the description, and not too strongly competing for visual attention with an underline (and at least for En-US users, in lower case), and the corresponding "less". Also like that it does so by fully unraveling it rather than just making the div scrollable.

My (purely visual, untested) feedback: I like YT's UI for this better, particularly that the click area is the full description box so it's easier to click, the "more" button is casual, placed inside the flow of the description, and not too strongly competing for visual attention with an underline (and at least for En-US users, in lower case), and the corresponding "less". Also like that it does so by fully unraveling it rather than just making the div scrollable.

Agreed. I went ahead and implemented your feedback in another branch. Here are screenshots of what I did. If this looks like its on a better track, I'll merge my new branch into this one.

The only piece here that I didn't do is make the description fully unravel. The reason is that for very long descriptions, the description becomes longer than the recommended videos component on the right. This causes the page to auto load recommended videos which makes the page longer, causing the description to partially scroll back up and makes scrolling to the bottom of the description difficult.

On load:

Hovering over the description (entire description is clickable while collapsed):

Clicked description:

Show less:

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Conflicts have been resolved. A maintainer will review the pull request shortly.

@kommunarr any additional thought on the way its implemented now?

Sorry for the delay, that looks great @sabs21! Please merge that in.

For other testers, here's a video with a short description, where it properly does not show the "Show more" button.

issue (pre-existing, non-blocking): seeing template errors for videos with no description (ex) due to no use of the null access operator.

issue: Sorry for all the different rounds of feedback, but I think this should be the last! I don't mind the "Show less" button being at the top, but it should have the same top padding as the p.description that it replaces.

I went ahead and changed the visual ordering of the show less button so that the padding is normal once again. I didn't notice that the show less button was ordered differently due to me messing around with CSS locally in the dev tools. The spacing now appears normal once again too.

As a byproduct of this change, clicking the Show more button now opens at the end of the container (i.e., scrolled all the way down) instead of at the top.

As a byproduct of this change, clicking the Show more button now opens at the end of the container (i.e., scrolled all the way down) instead of at the top.

I reverted the previous change and just added the margin to the top of the show less button

Anyone got a video with short description for testing?

Update 1: The one in one of the comments is removed

Just got error when trying to find a video with short description
https://youtu.be/ASeJV9KGAOc

Show Less is now seen in the top of the description box but i expected it to be shown. Was this an intentional change?